[Fix] `shallow`: `x.find()` should not change state of `x` #2061

sstern6 · 2019-03-19T16:10:05Z

Fixes #1916.

…dComponentUpdate` method before updating the wrapper. Fixes enzymejs#1916.

packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

packages/enzyme/src/ShallowWrapper.js

ljharb

I've rebased this.

I suspect we'll need to add shouldComponentUpdate to all the adapters.

ljharb · 2019-04-06T23:20:44Z

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

@@ -1785,6 +1785,7 @@ describe('shallow', () => {
        const wrapper = shallow(<MyComponent />, { disableLifecycleMethods: true });
        expect(wrapper.find(Table).length).to.equal(0);
        wrapper.instance().componentDidMount();
+        wrapper.update();


the need for this addition concerns me; perhaps that's OK because most people won't be manually calling a lifecycle method, but this makes it seem like it might be a breaking change :-/

@ljharb any idea or prediction on when this could be included in an official release? 👍

finishing this up today

i dont think we need this if we return true per your comment here: #2061 (comment). Checking though

sstern6 · 2019-04-10T01:54:42Z

re > I suspect we'll need to add shouldComponentUpdate to all the adapters.

If we have the conditional to check if the shouldUpdateComponent is present why would we need to include it in all adapters?

Edit: some tests are failing, going to look into that as well, pushed some changes per your comments.

sstern6 · 2019-04-10T02:02:49Z

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx

@@ -1785,6 +1785,7 @@ describe('shallow', () => {
        const wrapper = shallow(<MyComponent />, { disableLifecycleMethods: true });
        expect(wrapper.find(Table).length).to.equal(0);
        wrapper.instance().componentDidMount();
+


@ljharb was able to remove the wrapper.update() by returning true in the adapter function.

This still seems to be failing.

sstern6 · 2019-04-10T02:03:04Z

packages/enzyme/src/ShallowWrapper.js

    if (this[ROOT][WRAPPING_COMPONENT]) {
      this[ROOT][WRAPPING_COMPONENT].unmount();
+      this[ROOT][WRAPPING_COMPONENT].update();


Curious why these were added?

to make tests pass; they were failing.

ljharb · 2019-04-10T05:17:28Z

why would we need to include it in all adapters?

@sstern6 because the issue this fixes should be fixed for all versions of react, not just react 16.

plemasantos · 2019-05-06T10:14:26Z

Hey guys!

Any news on this PR?

lmvco · 2019-06-06T09:32:54Z

Hi guys,

I am also waiting for this PR. Is someone looking into this?

heldergoncalves92 · 2019-06-06T14:13:46Z

Do you have plans to release it? It is an important fix that I'm waiting for.

ljharb · 2019-06-06T15:53:59Z

@sstern6 ping for an update?

sstern6 · 2019-06-24T16:23:00Z

So sorry, didnt get pinged on my email. Yes I will prioritize this week.

I sincerely apologize, a lot of personal things going on right now has been hard to see this through.

If someone else would like to take it please be my guest. If not, this will be prioritized this week

munerf · 2019-06-26T15:48:38Z

I'm also interested in this fix

munerf · 2019-07-05T17:12:48Z

hi @sstern6, were you able to work on this? I need this, how can I help?

plemasantos · 2019-08-06T08:09:37Z

Hey again!

Sorry for pinging constantly, but are there any news?

codecov · 2020-12-21T02:46:54Z

Codecov Report

Merging #2061 (9ab718c) into master (0e39e1f) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2061      +/-   ##
==========================================
+ Coverage   96.12%   96.13%   +0.01%     
==========================================
  Files          49       49              
  Lines        4004     4015      +11     
  Branches     1123     1127       +4     
==========================================
+ Hits         3849     3860      +11     
  Misses        155      155

Impacted Files	Coverage Δ
...enzyme-adapter-react-16/src/ReactSixteenAdapter.js	`95.38% <100.00%> (+0.06%)`	⬆️
packages/enzyme/src/ShallowWrapper.js	`99.06% <100.00%> (+<0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e39e1f...9ab718c. Read the comment docs.

sstern6 added 2 commits January 14, 2019 18:30

[enzyme-adapter-react-16] [new] add shouldComponentUpdate

28766ef

[fix] shallow: getNodesInternal: delegate to the adapter’s `shoul…

9ab718c

…dComponentUpdate` method before updating the wrapper. Fixes enzymejs#1916.

sstern6 mentioned this pull request Mar 19, 2019

another .find() will cause the previous .find() not able to get .parent() #1916

Open

13 tasks

ljharb reviewed Mar 20, 2019

View reviewed changes

packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js Outdated Show resolved Hide resolved

packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx Outdated Show resolved Hide resolved

packages/enzyme/src/ShallowWrapper.js Outdated Show resolved Hide resolved

ljharb changed the title ~~Issue1916~~ [Fix] shallow: x.find() should not change state of x Mar 20, 2019

ljharb force-pushed the issue1916 branch from d166048 to 531e6fb Compare April 6, 2019 23:16

ljharb added API: shallow package: react adapter: 16 semver: minor New stuff. semver: patch labels Apr 6, 2019

ljharb reviewed Apr 6, 2019

View reviewed changes

sstern6 commented Apr 10, 2019

View reviewed changes

ljharb force-pushed the master branch 3 times, most recently from 40cc703 to 0a17404 Compare April 12, 2019 23:05

ljharb force-pushed the issue1916 branch from d6ab780 to 4507396 Compare April 23, 2019 00:17

ljharb force-pushed the issue1916 branch from 32b994d to afc82f7 Compare September 28, 2019 21:26

ljharb force-pushed the issue1916 branch from afc82f7 to 9ab718c Compare December 21, 2020 02:37

ljharb added the help wanted label Dec 21, 2020

ljharb force-pushed the master branch 4 times, most recently from 2227326 to 0d5ead7 Compare December 21, 2020 07:46

ljharb force-pushed the master branch 3 times, most recently from 43eb75e to 39e6b1f Compare November 3, 2022 21:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] `shallow`: `x.find()` should not change state of `x` #2061

[Fix] `shallow`: `x.find()` should not change state of `x` #2061

sstern6 commented Mar 19, 2019 •

edited by ljharb

Loading

ljharb left a comment

ljharb Apr 6, 2019

plemasantos Apr 9, 2019

sstern6 Apr 9, 2019

sstern6 Apr 10, 2019

sstern6 commented Apr 10, 2019 •

edited

Loading

sstern6 Apr 10, 2019

ljharb Apr 23, 2019

sstern6 Apr 10, 2019

ljharb Apr 10, 2019

ljharb commented Apr 10, 2019 •

edited

Loading

plemasantos commented May 6, 2019

lmvco commented Jun 6, 2019

heldergoncalves92 commented Jun 6, 2019

ljharb commented Jun 6, 2019

sstern6 commented Jun 24, 2019

munerf commented Jun 26, 2019

munerf commented Jul 5, 2019

plemasantos commented Aug 6, 2019

codecov bot commented Dec 21, 2020 •

edited

Loading

[Fix] shallow: x.find() should not change state of x #2061

Are you sure you want to change the base?

[Fix] shallow: x.find() should not change state of x #2061

Conversation

sstern6 commented Mar 19, 2019 • edited by ljharb Loading

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sstern6 commented Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb commented Apr 10, 2019 • edited Loading

plemasantos commented May 6, 2019

lmvco commented Jun 6, 2019

heldergoncalves92 commented Jun 6, 2019

ljharb commented Jun 6, 2019

sstern6 commented Jun 24, 2019

munerf commented Jun 26, 2019

munerf commented Jul 5, 2019

plemasantos commented Aug 6, 2019

codecov bot commented Dec 21, 2020 • edited Loading

Codecov Report

[Fix] `shallow`: `x.find()` should not change state of `x` #2061

[Fix] `shallow`: `x.find()` should not change state of `x` #2061

sstern6 commented Mar 19, 2019 •

edited by ljharb

Loading

sstern6 commented Apr 10, 2019 •

edited

Loading

ljharb commented Apr 10, 2019 •

edited

Loading

codecov bot commented Dec 21, 2020 •

edited

Loading